• Thoughts on creating classes for each effect? It would be a little more verbose, but could make parsing the code a little easier? e.g.

class BloomingPetals {
  constructor(p5, getCurrentPalette, colorFromPalette) {
    ...
  }

  init() { ... }
}

then in Effects.js, this.blooming_petals = new BloomingPetals(p5, getCurrentPalette, colorFromPalette)

@sanchitmalhotra126 , could you explain why this would be easier to parse?

@sanchitmalhotra126 , could you explain why this would be easier to parse?

It's just more modern syntax - the current format seems like it's the older pre-ES6 version of classes where you'd write it as a function that returns an object, but still use the this keyword to refer to properties and functions declared on the object. I personally find the class syntax a little clearer in displaying what the object properties and functions are, as well as what the this keyword refers to.

Also, thinking ahead to if we were to eventually start using TypeScript in this package, we could declare an interface/abstract class that these classes implement/inherit from. These could define the signatures for some common functions like init and draw for example.

@sanchitmalhotra126 , could you explain why this would be easier to parse?

It's just more modern syntax - the current format seems like it's the older pre-ES6 version of classes where you'd write it as a function that returns an object, but still use the this keyword to refer to properties and functions declared on the object. I personally find the class syntax a little clearer in displaying what the object properties and functions are, as well as what the this keyword refers to.

Gotcha - so it would be easier for humans to read.

Also, thinking ahead to if we were to eventually start using TypeScript in this package, we could declare an interface/abstract class that these classes implement/inherit from. These could define the signatures for some common functions like init and draw for example.

Cool - thanks for the explanation!

@sanchitmalhotra126 agree re: classes, was just easier to copy/paste if I stuck to the existing older syntax.

Re: ES6 import/export, I played with using import / export default and see these errors, is there a babel plugin we're missing or something?

Both of these do feel like work items that could be picked up as standalone follow-ups.

Oh this is interesting re: Babel config when using dance-party consumed by code-dot-org (this is from code-dot-org config):

// Using babel.config.json instead of .babelrc makes this configuration apply
// even when we are transpiling within node_modules.

https://github.com/code-dot-org/code-dot-org/blob/staging-next/apps/babel.config.json#L1-L2

Oh this is interesting re: Babel config when using dance-party consumed by code-dot-org (this is from code-dot-org config):

// Using babel.config.json instead of .babelrc makes this configuration apply
// even when we are transpiling within node_modules.

https://github.com/code-dot-org/code-dot-org/blob/staging-next/apps/babel.config.json#L1-L2

Just a note that our johnny-five fork also uses module.exports and require. Just curious if you learned anything more about this @bencodeorg